Ship shared plugin hooks - #550
Conversation
a480e3e to
6bc3e87
Compare
6bc3e87 to
9c1e036
Compare
There was a problem hiding this comment.
Pull request overview
This PR ships a shared hooks/hooks.json at the plugin root so both Claude Code and Codex can run the same hook configuration, enabling session-start status context and Git commit-reference nudges via basecamp agent-hook …. It also restores/updates setup guidance and docs so users can install the plugins and (for Codex) trust hooks via /hooks.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Add shared agent hook configuration in
hooks/hooks.json(SessionStart + Bash PreToolUse/PostToolUse/PostToolUseFailure). - Update Codex setup wizard output + tests to include
/hookstrust guidance before “start a new thread”. - Refresh documentation/manifests to mention hooks and commit-to-todo linking.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates AI agent integration section to include hooks and Codex /hooks trust guidance. |
| internal/commands/wizard_codex.go | Prints Codex post-setup next steps including trusting hooks before starting a new thread. |
| internal/commands/wizard_codex_test.go | Asserts /hooks trust guidance is present and ordered before “start a new thread”. |
| install.md | Restores hooks language and adds Codex /hooks trust guidance in install steps. |
| hooks/hooks.json | Introduces shared hook definitions invoking basecamp agent-hook … for session context and commit nudges. |
| AGENTS.md | Adds hooks/ to the repository tree overview. |
| .codex-plugin/plugin.json | Updates the Codex plugin longDescription to include commit-to-todo linking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c1e036141
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Landing sequence — read before marking this readyThis PR is now a draft so the gate is enforced rather than implied. The hooks call The order matters, and step 4 is the one thats easy to miss:
On (4): merging the hooks after v0.8.0 while leaving the plugin manifest at The wrapper alternative raised in the P1 thread was considered and rejected on purpose: an old CLI writes its error to stdout (exit 7), so a |
One hooks/hooks.json at the plugin root serves both agents: SessionStart reports Basecamp auth status as model context, and the paired PreToolUse/PostToolUse Bash hooks drive the HEAD-snapshot commit nudge via the hidden agent-hook command. PostToolUseFailure routes to the same post-commit check so a `git commit && git push` with a failed push still nudges on Claude; Codex ignores unknown event names inside hooks, and its PostToolUse already fires regardless of exit status. No other top-level keys — Codex hard-errors on them. The tool hooks carry no statusMessage: they run on every Bash call and stay visually silent. Commands are plain `basecamp agent-hook <sub>` strings, valid under all four hook shells (sh -lc, cmd.exe /C, Git Bash, PowerShell) with no wrapper or per-agent split. The dormant TestHooksFileCommandsInvokeBasecamp gate activates with this file. Restore the hooks language deferred in cb13972: README and install.md regain hooks in the plugin feature lists and the Codex /hooks trust step (Codex lists untrusted hooks but does not run them until trusted; Claude's install consent covers it), the AGENTS.md tree regains hooks/, the Codex manifest regains the connect-commits clause, and setup codex again points at /hooks. Hooks require a CLI release containing agent-hook. On an older CLI a refreshed plugin payload produces non-blocking unknown-command hook errors on either agent; remediation is `basecamp upgrade`.
…I floor The hook timeouts were set below what the commands can actually take. gitOutput gives every git invocation its own 2s deadline, and the two git-touching subcommands make several in sequence: pre-commit-snapshot agentHookHead up to 4 calls ~8s post-commit agentHookCommitReference up to 5 calls ~10s Both sat under a 5s outer timeout, so on a slow disk or filesystem the hook could be killed mid-run — dropping the snapshot, and with it the nudge that depends on it. Raised to 10s and 12s respectively, covering the worst case with margin. PostToolUseFailure runs the same post-commit command and gets the same 12s. SessionStart makes no git calls and stays at 5s. The docs now state the CLI floor the hooks imply. They call `agent-hook`, which older installs do not have, and the failure mode gave no hint: hook errors after a plugin refresh, with nothing pointing at the CLI version. README and install.md now name the requirement, give the remediation (`basecamp upgrade`, then a new session), and name the check — `basecamp agent-hook --help` answers "unknown command" on a CLI too old, which is exactly what an install in that state reports.
db3aad5 to
7e605dd
Compare
|
@codex review Rebased onto |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
feat: ship shared plugin hooks (
hooks/hooks.json) + docsMerge gate cleared — release N shipped as stable
v0.8.0on 2026-08-03, and it is Latest.This PR was held because a plugin installed from the default branch would have called
agent-hook, a command the then-publishedv0.7.2did not have. Verified against the shipped artifact rather than the source tree, since that is what users actually download:basecamp_0.8.0_darwin_arm64.tar.gzreportsbasecamp version 0.8.0and exposes exactly the three subcommandshooks/hooks.jsoninvokes (session-start,pre-commit-snapshot,post-commit). The full flow was then exercised end to end against that binary: snapshot → commitBC-4455→ nudge emitted with the reference and short hash.Rebased onto
main(was 45 commits behind);bin/ciexit 0.What
One
hooks/hooks.jsonat the plugin root serves both agents (both read the same location, same matcher semantics, samehookSpecificOutputwire):startup|resume|clear|compact) →basecamp agent-hook session-start— auth status as model context, with a spinner label.Bash) →pre-commit-snapshot/post-commit— the paired HEAD-snapshot commit nudge. NostatusMessageon these: they run on every Bash call and stay visually silent.Bash) →post-commit— closes the old accepted asymmetry: on Claude,git commit && git pushwith a failed push still nudges (the commit is proven from repo state, not the tool result). Codex ignores unknown event names insidehooks(verified:HookEventsTomllacksdeny_unknown_fields), so this key is inert there; Codex's own PostToolUse fires regardless of exit status. No other top-level keys — Codex hard-errors on unknown top-level keys in hooks.json.Commands are plain
basecamp agent-hook <sub>strings — valid under all four hook shells (sh -lc,cmd.exe /C, Git Bash, PowerShell); no wrapper, nocommandWindows, no per-agent split.Docs/setup restoration (reverses the Phase 1 deferrals from
cb139726):/hooks" (Codex lists untrusted hooks but silently does not run them until trusted). Claude needs no trust step — install consent covers it.runCodexSetup: re-adds the muted "Review and trust the plugin hooks with /hooks." next-step line; the wizard test flips from proving/hooksabsent to proving it present.hooks/entry back (agent-neutral wording)..codex-plugin/plugin.jsonlongDescription: restores the connect-commits clause.TestHooksFileCommandsInvokeBasecamp(dormant since #534) activates automatically now thathooks/hooks.jsonexists.Compatibility floor
Hooks require CLI ≥ release N. Users of either agent with an older CLI may see non-blocking hook errors after their plugin payload refreshes (Codex: on
marketplace upgrade; Claude: at the release N+1 manifest version bump, which can auto-update the plugin while the user still runs CLI < N). Remediation:basecamp upgrade. Codex additionally keeps new hooks inert until/hookstrust.Rollout
codex plugin marketplace upgrade 37signals(orbasecamp setup codex), then must trust via/hooks.Staging verification (pre-merge, candidate payload + dev-build CLI)
Both agents were verified against this branch's SHA (
4f28a810) with production plugins isolated first (Codex: throwawayCODEX_HOME+ staging marketplace37signals-stagingpinned to the SHA, install positively attributed; Claude: productionbasecamp@37signalsdisabled, candidate loaded via--plugin-dir), then production state restored.codex exec --dangerously-bypass-hook-trust, dev-buildbasecampon PATH): SessionStart context injected verbatim ("Basecamp is active and OAuth is ready…"); scratch repo commit withBC-123→ nudge delivered as developer context with the short hash; failed commit (nothing staged) → NONE./hookstrust flow itself not exercisable non-interactively — the bypass flag exercises the same execution path post-trust.PostToolUsenudge ontodo-456commit quoted back by the model;PostToolUseFailureon commit-then-failed-push quoted theBC-777nudge verbatim — the asymmetry this event closes, verified live. SessionStart verified at the transcript level: hook executed, stdout parsed, context attached ashook_additional_context(models don't always attribute it when asked, but injection is proven).TestHooksFileCommandsInvokeBasecampnow runs (not skipped) and passes; fullbin/cigreen on this branch.Summary by cubic
Ships shared plugin hooks for Codex and Claude via
hooks/hooks.jsonto add session-start status and Git commit-to-todo nudges. Widens hook timeouts and updates docs to require a CLI withagent-hook, plus trust-first guidance in Codex.New Features
hooks/hooks.jsonfor both agents:SessionStartinjects auth status; BashPreToolUse/PostToolUsedrive commit nudges;PostToolUseFailure(Claude) also nudges after a failed push; timeouts set to 5s/10s/12s.basecamp agent-hook <sub>across shells and stay silent on Bash. Codex ignores unknown hook event names and rejects unknown top-level keys.agent-hookCLI requirement; Codex setup now says “trust hooks with/hooks” before “start a new thread”;.codex-plugin/plugin.jsonmentions commit linking;AGENTS.mdlistshooks/.Migration
basecampCLI withagent-hook(release N+). If hook errors appear after install/refresh, runbasecamp upgrade, start a new session, and check withbasecamp agent-hook --help(“unknown command” means the CLI is too old)./hooks, then start a new thread. Claude needs no trust step.Written for commit 7e605dd. Summary will update on new commits.